Should probably validate that writes doesn't have a duplicate mapping, given that it's the values.

I changed some mappings to ensure reads={socket: state_field} and writes={state_field: socket}.

This seems to make the most sense because:

• on reads, each key should be unique and map to a unique kwarg of Component.run(). The same state value could be passed to different kwargs
• on writes, each key should be a unique state field. Even though Component.run() returns a dictionary where keys are unique, we want to prevent calling .update() on the same field several times.

In other words, this would be invalid:

class CustomComponent:
  def run() -> dict:
     return {"bar": 1, "baz": 2}

# mapping both `bar` and `baz` to `State["foo"]` is invalid
HaystackAction(
   component=CustomComponent()
   writes={"bar": "foo", "baz": "foo"}
)

# reverting the mapping ensures that two outputs can't be mapped to `foo`
HaystackAction(
   component=CustomComponent()
   writes={"foo1": "bar", "foo2": "baz"}
)

Yes, I think that makes sense, just make sure it's thoroughly documented

We should probably assign this in the constructor? Break it into a helper function.

Regarding .inputs(), it is currently a property, but it's return value may change if we allow a .bind() method. What was previously a required/optional input is now bound.

If .bound() is removed, then yes we could set values in __init__()

It seems that this logic belongs to .inputs() and wouldn't be of much use elsewhere.

Yeah, .bound() should return a copy of the object so the .inputs() are frozen IMO. That said, this should take in bound_inputs in the constructor, and we don't need a method? You don't really want properties dynamically computed like this, it's hard to debug and a bit iffy IMO.

So bind is only (currently) for functions, actions don't have it. Might be a bit confusing here, but if we like it, it should really be at the Action level, as that makes a lot of sense IMO. Then it could just do this.

Otherwise we can make this a more specific name to not conflict

If I understand, the pattern is to only allow bound_params at init? This affects the Component.run() method

This would be ok:

haystack_retrieve_documents = HaystackAction(
    component=InMemoryEmbeddingRetriever(InMemoryDocumentStore()),
    name="retrieve_documents",
    reads=["query_embedding"],
    writes=["documents"],
    bound_params={"foo": "bar"},
)

This would not be ok?

haystack_retrieve_documents = HaystackAction(
    component=InMemoryEmbeddingRetriever(InMemoryDocumentStore()),
    name="retrieve_documents",
    reads=["query_embedding"],
    writes=["documents"],
)
haystack_retrieve_documents.bind(foo="bar")

I should simply remove the .bind() method?

I think so, or call it something else? Alternatively, we can make bind work at the action level. Currently it's only for functions, and class-based actions don't have it.

So yeah, the pattern should be a copy-operation here, or remove this and just do the inputs in the parameters.

printline

Yes, I think that makes sense, just make sure it's thoroughly documented

So yeah, the pattern should be a copy-operation here, or remove this and just do the inputs in the parameters.